feat: add api endpoint to generate and serve cached file #1
Conversation
For easier tests
Queue system enable back the cache file generation on demand when cache is not available on fetch
There was a problem hiding this comment.
I get this error, I did yarn install and yarn dev then i see this error
snapshot-sidekick/node_modules/ts-invariant/lib/invariant.js:11
var _this = _super.call(this, typeof message === "number"
^
Invariant Violation:
"fetch" has not been found globally and no fetcher has been configured. To fix this, install a fetch package (like https://www.npmjs.com/package/cross-fetch), instantiate the fetcher, and pass it into your HttpLink constructor. For example:
import fetch from 'cross-fetch';
import { ApolloClient, HttpLink } from '@apollo/client';
const client = new ApolloClient({
link: new HttpLink({ uri: '/graphql', fetch })
});
I think we should add cross-fetch as a dependency
|
Native node fetch is only available from node 18 |
Hmm, we should restrict this to node 18 only with node engines in package.json, or add cross-fetch if we want to support other nodejs version |
|
Added, make sense to always enforce node version, as the CI is only testing on node 18 |
|
Updated Readme (not sure how to suggest so many changes so added a commit, feel free to modify if required) |
|
Readme update seems good |
|
If you have your own aws api keys for S3, please also do test that. The keys provided for the production were returning an error, but my personal one do work. Prod will use AWS |
...snapshot-votes-report-0x1e5fdb5c87867a94c1c7f27025d62851ea47f6072f2296ca53a48fce1b87cdef.csv
Outdated
Show resolved
Hide resolved
src/helpers/utils.ts
Outdated
| } | ||
|
|
||
| export function voteReportWithStorage(id: string) { | ||
| return new VotesReport(id, new StorageEngine('votes')); |
There was a problem hiding this comment.
Can we add this votes at the config level, or maybe a global variable in this file? In case we want to change the folder name in future
There was a problem hiding this comment.
I mean a config file like config.ts not needed in .env
There was a problem hiding this comment.
I did go the .env route. Allow dev to each have their own preferences, without having unstaged changes in the repo. Also can customize the storage engine on prod or staging server, without having to edit the source code. wdyt?
|
|
||
| get = async (key: string) => { | ||
| try { | ||
| const command = new GetObjectCommand({ |
There was a problem hiding this comment.
maybe we can use putObject and getObject like in other repos?
https://github.com/snapshot-labs/subgrapher/blob/main/src/aws.ts#L22
There was a problem hiding this comment.
Other repos are using the old v2 sdk. In the long term, I think we should just move over to the new v3 syntax everywhere.
We're already using the v3 modularized version of their SDK (@aws-sdk/client-s3), why not also follow the recommended syntax that's shipped with it.
Their new API provides new features, like the transformToString() method on the response's body, saving us from using a custom streamToString function (https://github.com/snapshot-labs/subgrapher/blob/32d43b22603662e7410a9e0be8d62faeca5f233c/src/aws.ts#L11)
For consistency with other projects
|
@samuveth We should avoid untested review and merge should be done by the author of the PR. This PR require new conf vars for example I dont think it should have been merged that fast. |
|
@samuveth It is not a very urgent issue on Snapshot, right? It is not just about testing/issues 😄 we should also check the code, right? else we could be in trouble in the future/can take more time to understand everything. ideally, everyone in the team should be able to work on changes on this very easily (not saying this PR code is bad 😄 but generally speaking) Of course, I agree I was bit busy with other things and delayed this but I believe we should give more time to reviews. @aurelianoB please suggest some flow and schedule for reviewers :) maybe teams should leave a few weekdays to just test/review/rework? right now it is hard to plan/schedule reviews 🤔 because we never know what the other team is working on and when they request a review, also it is hard to know their priorities |
|
Sorry guys, shouldn't have merged this myself you are right. @ChaituVR Regarding the review schedule, I make it a point to check for requested reviews twice daily - once in the morning and once in the evening. I aim to complete these reviews promptly. In my view, it's important to prioritize reviews at least once a day, as this approach can effectively streamline project progress and ensure that team members receive timely feedback. |
Changes
This PR adds the endpoint to serve/generate a cached file.
In case the cached file does not exist yet, a generation task will be triggered, and a PENDING_GENERATION response will be returned instead.
There's also a dedicated endpoint to trigger the cache file generation
A queue system is used for cache file generation:
Todo
Ideally, we should have another background task to listen to new closed proposal events, and trigger the file generation. Waiting for feat: extract hub message-to-events translation logic to an external library snapshot-webhook#48Use snapshot's webhook service to retrieveproposal/closeevents/votes/generate/endpoint behind authentication, to avoid spam